Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CLI tab completions #741

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Ahmedsaed
Copy link
Contributor

@Ahmedsaed Ahmedsaed commented Aug 11, 2024

What does this PR do? Please describe:
This PR extends the CLI with support for tab completions. Initially, I tried using argcomplete, but it was slow and ran a Fairseq2 process each time I pressed tab, which was far from ideal.

I then discovered shtab, a package that generates shell tab completion scripts efficiently. According to its README:

  • What: Automatically generate shell tab completion scripts for Python CLI apps
  • Why: Speed & correctness. Alternatives like argcomplete and pyzshcomplete are slow and have side-effects
  • How: shtab processes an argparse.ArgumentParser object to generate a tab completion script for your shell

After implementing shtab, tab completion works beautifully. Here are some examples:

$ fairseq2 [TAB]
assets        llama         lm            mt            wav2vec2_asr  
$ fairseq2 ll[TAB]
$ fairseq2 llama

It’s especially useful for things like presets. Instead of listing presets and copying them manually, you can now use tab completion:

$ fairseq2 lm [TAB]
chatbot               generate              instruction_finetune  preference_finetune   
$ fairseq2 lm c[TAB]
$ fairseq2 lm chatbot

It also works with arguments:

$ fairseq2 lm chatbot --[TAB]
--cluster               --help                  --model                 --temperature           --top-p                 
--dtype                 --max-gen-len           --seed                  --tensor-parallel-size  

Tab completions are specific to each shell and require the installation of corresponding scripts in the shell's directory.

Initially, I intended to extend the project by adding post-install scripts to automatically set up tab completion scripts (b63af98). However, I discovered that this approach only worked with zip and tarball distributions, while fairseq2 uses wheel builds. As a result, I reverted that change.

Currently, you can enable tab completions manually by using --print-completions {bash|zsh|tcsh}. The output of this command can be saved to the shell's completion scripts directory for persistence or evaluated directly for the current session.

For example:

eval "$(fairseq2 --print-completion bash)"

Please note: each time the CLI interfaces are modified, the shell completion scripts need to be updated.

Does your PR introduce any breaking changes? If yes, please list them:
None.

Check list:

  • Was the content of this PR discussed and approved via a GitHub issue? (no need for typos or documentation improvements)
  • Did you read the contributor guideline?
  • Did you make sure that your PR does only one thing instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests?
  • Did you verify new and existing tests pass locally with your changes?
  • Did you update the CHANGELOG? (no need for typos, documentation, or minor internal changes)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 11, 2024
@Ahmedsaed
Copy link
Contributor Author

Ahmedsaed commented Aug 11, 2024

Most shells provide path completions by default. However, when using custom shell completions, this default behavior is overridden. To specify that an argument should use path completions, you need to use shtab.FILE for file completions and shtab.DIRECTORY for directory completions.

For example:

parser.add_argument(
    "input_dir",
    type=Path,
    help="Checkpoint directory",
).complete = shtab.DIRECTORY  # type: ignore[attr-defined]
parser.add_argument(
    "--config-file",
    dest="config_files",
    metavar="CONFIG_FILE",
    type=Path,
    nargs="*",
    help="YAML configuration file(s)",
).complete = shtab.FILE  # type: ignore[attr-defined]

This configuration ensures that input_dir will provide directory completions and --config-file will provide file completions. I have updated the whole codebase to correctly handle path completions.

I also had to ignore the type hints as I can't seem to find a better way to handle them.

@cbalioglu
Copy link
Contributor

Neat! Happy to merge this once ready. Any reason why it is draft at the moment?

@Ahmedsaed
Copy link
Contributor Author

Neat! Happy to merge this once ready. Any reason why it is draft at the moment?

I was waiting for further instructions on how to address the post-install issue. If we are not going to handle the shell completions setup automatically (i.e. post install code). Should I update some docs like INSTALL_FROM_SOURCE.md with instructions on how to setup and enable shell completions?

@Ahmedsaed Ahmedsaed marked this pull request as ready for review August 12, 2024 23:24
@Ahmedsaed Ahmedsaed requested a review from cbalioglu as a code owner August 12, 2024 23:24
@Ahmedsaed
Copy link
Contributor Author

Should I add support for PowerShell and fish shell (if possible)?

Currently supported shells are bash, zsh and tcsh. Which supports almost all Linux systems and macos

@cbalioglu
Copy link
Contributor

I was waiting for further instructions on how to address the post-install issue. If we are not going to handle the shell completions setup automatically (i.e. post install code). Should I update some docs like INSTALL_FROM_SOURCE.md with instructions on how to setup and enable shell completions?

Considering that our CLI commands are populated dynamically during runtime based on the packages installed in the Python environment, the safest would be to let users manually update their bashrc/zhrc once they have their environment fully set up. So adding some brief instructions in README (right after "Installing from Source") makes sense to me.

Should I add support for PowerShell and fish shell (if possible)?

I don't think they are necessary. PowerShell is mainly used on Windows. I haven't seen anyone using it on Linux/macOS. As long as we support bash and zsh, the rest is not really much relevant nowadays.

@Ahmedsaed
Copy link
Contributor Author

I have added a section in the Readme file on setting up shell completions in 4af2be4. This PR should be ready for merging now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants